Add Droid SDK provider#2689
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size:XL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
github-actions
Bot
added
size:XXL
|
Latest Droid follow-up fixes pushed in c72f19b:
Validation: |
|
Addressed the unresolved Droid cleanup review thread in b1dabf8. Added a regression test covering two resumed Droid sessions where one Validation:
Note: the first full-suite run hit a timeout in |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces a new provider integration (Droid SDK) with substantial new runtime logic, session management, and UI changes - a new feature requiring human review. Additionally, 3 unresolved review comments identify potential concurrency and logic bugs. You can customize Macroscope's approvability policy. Learn more. |
|
Split the Droid adapter by responsibility in af7ffb0:
The Droid implementation is now 989 lines total across focused modules instead of one nearly 900-line adapter file. Validation:
|
|
Addressed the two remaining Cursor Bugbot review threads in 8939e57:
Validation:
|
|
Addressed the remaining token-usage review thread in 742ab64:
Validation after this fix:
Earlier in this pass, full |
|
Follow-up fixes for the latest automated review threads:
Validation:
|
|
Follow-up for the latest Cursor Bugbot token usage thread:
Validation after this change:
|
|
Fixed the latest Cursor Bugbot thread:
Validation:
|
|
Addressed the latest Droid review batch in
Validation: |
|
Resolved the latest Bugbot findings in 969aceb.
Validation on this commit:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 969aceb. Configure here.
| context.activeAbort = undefined; | ||
| } | ||
| } | ||
| }).pipe(Effect.forkDetach); |
There was a problem hiding this comment.
Detached streaming fiber races with queue shutdown in finalizer
Medium Severity
The streaming turn runs in an Effect.forkDetached fiber, making it immune to scope interruption. When the finalizer aborts contexts and then immediately calls Queue.shutdown(runtimeEvents), the detached fiber may still be executing its catch/finally handler. If completeInterruptedTurn or completeFailedTurn calls emitNow after the queue is shut down, Queue.offer fails and runPromise produces an unhandled promise rejection, since there's no outer try-catch wrapping the awaited emitNow calls inside the catch block.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 969aceb. Configure here.
| resolvedUnlockedProvider !== undefined); | ||
| const runtimeMode = canNormalizeRuntimeMode | ||
| ? normalizeRuntimeModeForProvider(selectedProvider, rawRuntimeMode) | ||
| : rawRuntimeMode; |
There was a problem hiding this comment.
Provider resolution doesn't check enabled status for mode normalization
Medium Severity
resolveProviderDriverKindForInstanceSelection returns a driver kind even for disabled provider instances, unlike the old resolveSelectableProvider which verified requestedEntry?.enabled before returning. When a user has a disabled Droid instance selected, resolvedUnlockedProvider resolves to "droid", canNormalizeRuntimeMode becomes true, and medium-access mode is preserved even though the Droid provider cannot execute turns. This persists a provider-specific runtime mode that other enabled providers don't support.
Reviewed by Cursor Bugbot for commit 969aceb. Configure here.
| if (context.session.status === "running" || context.activeAbort) { | ||
| return yield* new ProviderAdapterValidationError({ | ||
| provider: DROID_PROVIDER, | ||
| operation: "sendTurn", | ||
| issue: `Droid thread ${input.threadId} already has an active turn.`, | ||
| }); | ||
| } | ||
| const text = input.input?.trim(); | ||
| const images = yield* resolveDroidImages(input.attachments ?? [], { | ||
| attachmentsDir: serverConfig.attachmentsDir, | ||
| fileSystem, | ||
| }); | ||
| if (!text && images.length === 0) { | ||
| return yield* new ProviderAdapterValidationError({ | ||
| provider: DROID_PROVIDER, | ||
| operation: "sendTurn", | ||
| issue: "Droid turns require text input or at least one attachment.", | ||
| }); | ||
| } | ||
|
|
||
| const turnId = TurnId.make(`droid-turn-${randomUUID()}`); | ||
| const abort = new AbortController(); | ||
| context.activeAbort = abort; | ||
| context.activeAssistantItems = new Map(); | ||
| context.activeThinkingItems = new Map(); | ||
| context.activeCompletedAssistantItems = new Set(); | ||
| context.activeTurnError = undefined; | ||
| context.activeTokenUsage = undefined; | ||
| context.activeTokenUsageBaseline = context.cumulativeTokenUsage; | ||
| context.turns.push({ id: turnId, items: [] }); |
There was a problem hiding this comment.
🟡 Medium Layers/DroidAdapter.ts:262
In sendTurn, the guard check at lines 262-268 validates context.session.status !== 'running' and !context.activeAbort, but yield* resolveDroidImages(...) at lines 270-273 can suspend the effect before activeAbort is assigned at line 284. If two concurrent calls enter for the same threadId, both pass the guard before either sets activeAbort, causing overlapping turns to corrupt shared state (activeAssistantItems, activeThinkingItems, activeTokenUsage). Consider using a synchronous atomic operation (like a compare-and-swap on activeAbort) to ensure only one caller proceeds.
- if (context.session.status === "running" || context.activeAbort) {
+ if (context.activeAbort) {
return yield* new ProviderAdapterValidationError({
provider: DROID_PROVIDER,
operation: "sendTurn",
- issue: `Droid thread ${input.threadId} already has an active turn.`,
+ issue: `Droid thread ${input.threadId} is busy (active turn or abort in progress).`,
});
}
+ const abort = new AbortController();
+ context.activeAbort = abort;
const text = input.input?.trim();🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/provider/Layers/DroidAdapter.ts around lines 262-291:
In `sendTurn`, the guard check at lines 262-268 validates `context.session.status !== 'running'` and `!context.activeAbort`, but `yield* resolveDroidImages(...)` at lines 270-273 can suspend the effect before `activeAbort` is assigned at line 284. If two concurrent calls enter for the same `threadId`, both pass the guard before either sets `activeAbort`, causing overlapping turns to corrupt shared state (`activeAssistantItems`, `activeThinkingItems`, `activeTokenUsage`). Consider using a synchronous atomic operation (like a compare-and-swap on `activeAbort`) to ensure only one caller proceeds.
Evidence trail:
apps/server/src/provider/Layers/DroidAdapter.ts lines 253-296 (guard at 262, yield at 270, state mutation at 284-296); apps/server/src/provider/droid/DroidAttachmentResolver.ts lines 23-71 (resolveDroidImages performs file I/O via fileSystem.readFile at line 53, creating an async suspension point)
2 participants
Intention
Add Droid as a first-class T3 Code provider using Factory's TypeScript SDK: https://github.com/Factory-AI/droid-sdk-typescript
This is still WIP while we validate more real Droid permission, file, MCP, auth, and long-running streaming flows.
What this adds
droidto the shared provider, model, settings, runtime source, and driver contracts.createSession/resumeSessionfrom@factory/droid-sdkin a T3 provider adapter.gif,jpeg,png,webp) as base64 SDK image sources resolved through the existing attachment store.initResult.availableModels, including user/custom models, while preserving custom model ids so duplicate underlying models remain selectable.Implementation shape
The Droid adapter has been split by responsibility instead of keeping one large file:
DroidAdapter.ts: session lifecycle and adapter orchestration.provider/droid/DroidRuntimeEvents.ts: SDK message to T3 runtime event projection.provider/droid/DroidSdkMappings.ts: SDK model, access, reasoning, usage, approval, and user-input mappings.provider/droid/DroidAttachmentResolver.ts: attachment MIME validation and image loading.provider/droid/DroidAdapterTypes.ts: shared Droid adapter types.Security / safety notes
droid; prompt content is sent through the SDK rather than shell interpolation.Validation
droid exec --model glm-5.1 --cwd /tmp ...returneddroid-pong.HomeLab - GLM-5.1,HomeLab - Trinity-Large-Thinking,Direct - GPT-5.5-Fast-xHigh, andDirect - GPT-5.5-Low.numTurns; streaming, thinking, access-mode mapping, and custom model discovery were updated.bun fmtpassed.bun lintpassed with existing unrelated web warnings.bun typecheckpassed.cd apps/server && bun run test src/provider/Layers/DroidAdapter.test.ts src/provider/Layers/DroidProvider.test.ts.bun run testpreviously passed; after rebasing, one full parallel run hit three unrelated web timeout failures, and rerunning those exact files in isolation passed.Note
Add Droid SDK provider with session management, streaming turns, and UI integration
DroidDriverandDroidAdapterimplementing session start/resume, streamed turns with image attachments, permission/user-input prompting, turn interruption, and session lifecycle management.checkDroidProviderStatusto probe CLI availability and discover models dynamically; surfaces a pending/warning draft when the CLI is absent or timing out.medium-accessruntime mode mapped to Droid's Medium autonomy level; the composer and access mode menu now render provider-specific mode options viagetRuntimeModeOptions/getRuntimeModeConfig.DroidSettingsto the settings schema, registersDroidDriverin the built-in driver list, and exposes the provider in the model picker sidebar with a 'new' badge.TextGenerationError.Macroscope summarized 969aceb.
Note
Medium Risk
Adds a new first-class provider with process execution, streaming runtime-event projection, and approval/user-input routing; issues are mostly isolated but could impact session lifecycle and UI mode selection.
Overview
Introduces Droid as a new provider backed by
@factory/droid-sdk, wiring it into server settings/contracts, provider registry, and UI provider/model pickers.On the server, adds
DroidDriverplus aDroidAdapterthat manages session start/resume/stop/interrupt, maps SDK stream messages into canonical runtime events (content/tool/token-usage/auth/MCP/errors), resolves image attachments, and performs provider health/model discovery via thedroid --versionCLI andavailableModels.Adds a new
RuntimeModevalue,medium-access, exposed only for Droid in the UI and normalized away when switching to other providers; includes tests for the adapter/provider mappings and updates provider icons/metadata defaults.Reviewed by Cursor Bugbot for commit 969aceb. Bugbot is set up for automated code reviews on this repo. Configure here.